Run all controllers in the same process #313
Conversation
google-prow-robot
added
the
size/M
grantr
force-pushed
the
combine-controllers
branch
from
bfcaf60 to
3f3598d
Compare
|
/retest |
|
/assign scothis
For initial review
/assign evankanderson
For approve
…On Thu, Aug 2, 2018, 4:40 PM Google Prow Robot ***@***.***> wrote:
@grantr <https://github.com/grantr>: The following test *failed*, say
/retest to rerun them all:
Test name Commit Details Rerun command
pull-knative-eventing-integration-tests 3f3598d
<3f3598d>
link
<https://gubernator-internal.googleplex.com/build/knative-prow/pr-logs/pull/knative_eventing/313/pull-knative-eventing-integration-tests/283/> /test
pull-knative-eventing-integration-tests
Full PR test history
<https://gubernator-internal.googleplex.com/pr/knative_eventing/313>. Your
PR dashboard <https://gubernator-internal.googleplex.com/pr/grantr>.
Instructions for interacting with me using PR comments are available here
<https://git.k8s.io/community/contributors/guide/pull-requests.md>. If
you have questions or suggestions related to my behavior, please file an
issue against the kubernetes/test-infra
<https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:>
repository. I understand the commands that are listed here
<https://go.k8s.io/bot-commands>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#313 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHlyNxIj4ZsklSPlyq0ySMsOCQUYmipEks5uM43xgaJpZM4VtIaf>
.
|
|
/lgtm |
|
This is LGTM to me as well, thanks for doing this! |
| // crMain runs controllers written for controller-runtime. It's intended to be | ||
| // called from main(). As controllers are migrated to use controller-runtime, | ||
| // their initialization should be moved to this function. | ||
| func crMain() error { |
There was a problem hiding this comment.
I am not in-love with crMain, Perhaps controllerRuntimeStart or something more descriptive without main
The other thing I have seen is _main to be the thing called after some setup.
There was a problem hiding this comment.
👍 I like controllerRuntimeStart. Renamed.
cmd/controller/main.go
Outdated
| // Start the controller-runtime controllers. | ||
| go func() { | ||
| if err := crMain(); err != nil { | ||
| glog.Fatalf("Error running controller-runtime controllers: %v", err) |
There was a problem hiding this comment.
you should have access to logger now from the zap setup that was merged.
There was a problem hiding this comment.
looks like you need to rebase
| @@ -1,33 +0,0 @@ | |||
| # Copyright 2018 The Knative Authors | |||
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: grantr If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
grantr
force-pushed
the
combine-controllers
branch
from
41974b7 to
db2fc1f
Compare
google-prow-robot
added
size/L
controller-runtime implements its own command wrapper, but it can still run in the same process as other controllers. This moves them to a single deployment and removes the existing cmd/controller-manager binary.
grantr
force-pushed
the
combine-controllers
branch
from
db2fc1f to
3be0819
Compare
|
Rebased and RFAL @n3wscott. |
|
/lgtm |
|
@n3wscott More conflicts! Merged this time (the merge commit will be squashed out). RFAL. |
|
/lgtm This is going to be grand!!! |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson, grantr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
* Update reconciler-test to latest commit in release-1.11 * Include latest 1.11 commits
controller-runtime implements its own command wrapper, but it can still run in the same process as other controllers. This moves them to a single deployment to keep logs for all controllers in one place.
Since controller-runtime defines its own
kubeconfigandmasterflags, we have to remove them from theinithere, but they should still work the same (since flags are global).Fixes #309
Proposed Changes
controller-managerdeployment.eventing-controllerpod./cc @pmorie